feat(svelte-form): Add Svelte adapter #1247
Conversation
Absolutely untested, written without running it or trying it out in any way. Just a very rough start written off the top of my head with the help of various bits of docs.
- one Field component with a module script with createField in it - createForm file - bunch of types that pass along generics, closely modeled after the other adapters - tests
|
View your CI Pipeline Execution ↗ for commit 55e5c82.
☁️ Nx Cloud last updated this comment at |
| </div> | ||
| </form> | ||
|
|
||
| <pre>{JSON.stringify(form.state, null, 2)}</pre> |
There was a problem hiding this comment.
This doesn't seem to be reactive, even replacing it with form.state.values.firstName doesn't update
There was a problem hiding this comment.
This is a leftover from some early explorations - I just removed it. FWIW for Solid and Vue this is also unreactive, from what I understand you gotta use form.useStore(...) to get a reactive version of (a part of) the state.
|
Let me know if you want any help with this 👍 my pr went stale since I was in need of someone to pair/review with but I am still very much interested in landing this |
| ], | ||
| "dependencies": { | ||
| "@tanstack/form-core": "workspace:*", | ||
| "@tanstack/svelte-store": "^0.7.0" |
There was a problem hiding this comment.
this should come from workspace:* I think
There was a problem hiding this comment.
This isn't part of the monorepo, so like this is the correct way
| } from '@tanstack/form-core' | ||
| import { useStore } from '@tanstack/svelte-store' | ||
| import { onMount, type Snippet } from 'svelte' | ||
| import Field from './Field.svelte' |
There was a problem hiding this comment.
this is really, really odd. importing the file we're in, from the file we're in
i'm surprised this did something. even if this somehow works, we really shouldn't be doing it imo
this is why createField and Field were originally separate
There was a problem hiding this comment.
Why shouldn't we? This is valid JavaScript, you can self-import in ESM, it's part of the spec. This is just a different way of doing what the other adapters are doing, where they have Field and createField in the same file.
There was a problem hiding this comment.
its basically a hacky workaround for the fact there's a circular dependency here (createField needs Field and vice versa), and in svelte the component is the module as a whole
ideally, we'd figure it out properly. though im not sure how off the top of my head
There was a problem hiding this comment.
Are you running into a problem with the circular ref? In general it's ok due to the way js imports work (e.g., you can import yourself), but ideally something we resolve.
There was a problem hiding this comment.
im fully aware that this "works", its just not great for code quality/readability and is inconsistent with the rest of the codebase
also its a clear workaround for the circular dependency. you did this because you couldn't define Field in the file itself (since it is the file), whereas in all other integrations we would just have it in a named export
i disagree with doing it this way but will leave it up to other reviewers to decide
There was a problem hiding this comment.
You're not gonna get a "clean" solution because there's a cyclic dependency here. So either you have a cyclic import or you have a self-import - I prefer the latter due to colocation.
| extendedApi.createField = (props) => | ||
| createField(() => { | ||
| return { ...props(), form: api } | ||
| }) as never |
There was a problem hiding this comment.
how come you weren't able to type this properly? (i.e. without as never)
There was a problem hiding this comment.
added a comment; TS throws an error here that type instantiation is excessively deep (same as for Solid FWIW)
|
Is this ready for testing it in a project? |
And if it is, would love to know if there is a nightly build :), Otherwise what is left / where we can help. |
I see an error in types that has an @ts/error with no actual error, and as someone else pointed that import failing. I think the reviews need to approve and best guess they will once those two failing tests pass from the fixes I mentioned... |
This implements a Svelte adapter for the form package. API-wise it's somewhere inbetween Solid and React (
createFormis used like Solid's, getting the field state from within a snippet is more like React). Added basic tests and two examples.Superseeds #1107